Code Review

Overview

This PR fixes an intermittent bug where mix-blend-mode: multiply on the hillshade layer drops out at max zoom. The root cause is correctly diagnosed: the inner .leaflet-tile-container's transform: translate3d creates a new stacking context that can isolate the container-level blend from adjacent layers. Moving the rule to img.leaflet-tile sidesteps this by blending each tile image directly in the parent pane's stacking context.

Code Correctness

The fix is technically sound. This is a well-known CSS compositing edge case — transform on a child establishes a stacking context that breaks out-of-flow blending with siblings. Applying mix-blend-mode at the leaf (img) level is the right workaround.

One alternative worth knowing about: adding isolation: isolate to the parent .leaflet-tile-pane could also prevent the stacking context from escaping. However, that would change blending behavior across all layers in the pane, not just hillshade. The tile-image approach is more surgical and correct.

Minor concern: .leaflet-tile is a Leaflet-internal class. If Leaflet renames it in a future version this will silently break. The risk is low (Leaflet 1.x has been stable here for years), but worth a note in the project's Leaflet upgrade checklist if one exists.

Inline Comment Weight

The added comment block (3 lines) is heavier than CLAUDE.md's convention of a single brief line only when the WHY is non-obvious. The stacking context issue is non-obvious, so a comment is justified — but the detail level here duplicates the PR description. Consider trimming to a single line:

/* Each tile blends individually — a container-level blend isolates under translate3d zoom animation. */
.hillshade-blend img.leaflet-tile { mix-blend-mode: multiply; }

Test Checklist Discrepancy

The PR description checks off:

npm test clean (628 tests pass)

Per CLAUDE.md, this project has no test framework and no test script in package.json. Either the checklist was copied from a template without adjustment, or package.json has been updated since CLAUDE.md was written. Either way, this item should be corrected — if tests don't exist, remove the checkbox; if they do, update CLAUDE.md to reflect it.

Summary

The core change is a clean 1-line fix with solid reasoning. Addressing the checklist discrepancy and trimming the comments would bring it fully in line with project conventions.

Cycle-2 fixes in the latest push:

• Comment trimmed to one line per project convention.
• CLAUDE.md refreshed — npm test now listed (vitest, 628 tests), recording replaced by guidance in features, file-structure list updated with guidance.ts / routing.ts / geo.ts / layers-control.ts / consent.ts / keepalive.ts. The `No automated tests" note is gone — that was the staleness you flagged.

Code Review

Overview

This PR fixes a visual bug (#168) where the hillshade multiply blend drops out at max zoom levels. The fix moves mix-blend-mode: multiply from the Leaflet layer container (.hillshade-blend) down to individual tile images (.hillshade-blend img.leaflet-tile). The PR also updates CLAUDE.md to reflect the current project state.

CSS Fix (src/style.css)

The fix is correct and the root cause analysis is accurate.

/* Before */
.hillshade-blend { mix-blend-mode: multiply; }

/* After */
.hillshade-blend img.leaflet-tile { mix-blend-mode: multiply; }

The stacking context isolation issue is a well-known CSS compositing behavior: when a descendant uses transform, it creates a new stacking context that causes mix-blend-mode on an ancestor to composite against an opaque backdrop rather than blending with true siblings. Placing the blend on the leaf <img> elements avoids this entirely.

Observations:

• The selector .hillshade-blend img.leaflet-tile is appropriately scoped — only hillshade tiles are targeted, no spillover risk.
• The updated comment (Per-tile multiply — a container-level blend can isolate under .leaflet-tile-container's translate3d.) is concise and captures the why, consistent with project conventions.
• Minor performance note: Applying mix-blend-mode to N tile images instead of one container means the GPU composites N blends rather than one. In practice, modern browsers GPU-accelerate this well and the tile count (~16–25 visible at any time) makes this negligible. Worth knowing, but not a concern here.
• No regression risk on other basemaps: The fix only affects elements under .hillshade-blend, which is only added when the hillshade overlay is active. Streets/CyclOSM/Humanitarian are unaffected.

CLAUDE.md Updates

The documentation updates are accurate and useful:

• New features (navigation, PWA caching, consent modal, wake lock) are correctly documented.
• The test command (npm test) and size-limit command (npm run size) are valuable additions.
• File structure is updated to reflect the current module list.
• Removing "No automated tests" and replacing it with the correct vitest + lint + type-check workflow is a good correction.
• ADR links in the architecture section are a nice addition for deeper context.

One small inconsistency to consider: the PR test plan references npm test passing 628 tests, but the CLAUDE.md test command note says "vitest for unit tests (pure-function tests; no DOM/Leaflet integration)". Both are fine as-is; just noting the manual browser verification items are unchecked — that's expected for a visual bug fix and not a blocker.

Summary

Verdict: Ready to merge after manual verification is completed. The CSS change is minimal, well-reasoned, and the documentation updates are a net positive.

🤖 Generated with Claude Code